Skip to content

Fixed flaky file-source unit tests #6984#6995

Merged
dlvenable merged 1 commit into
opensearch-project:mainfrom
srikanthpadakanti:fix/flaky-file-source-tests-6984
Jul 21, 2026
Merged

Fixed flaky file-source unit tests #6984#6995
dlvenable merged 1 commit into
opensearch-project:mainfrom
srikanthpadakanti:fix/flaky-file-source-tests-6984

Conversation

@srikanthpadakanti

Copy link
Copy Markdown
Collaborator

Description

The tests relied on wall-clock timing and background threads, so they passed locally but failed randomly on CI. I made them deterministic:

  • Injected a java.time.Clock into FileReader so the read/drain timeout logic can be driven by a fake clock in tests instead of real time. Removed all Thread.sleep and tiny deadlines.
  • Made the pool tests use a non-running executor so queueing and promotion counts are checked synchronously, not against a reader completing on a background thread.

No product behavior change; the default clock is still the system clock.

Issues Resolved

Resolves #6984

Check List

  • [ X ] New functionality includes testing.
  • New functionality has a documentation issue. Please link to it in this PR.
  • New functionality has javadoc added
  • [ X ] Commits are signed with a real name per the DCO

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

…lable executor (opensearch-project#6984)

FileReader.readLoop and its backpressure/batch logic branched on
System.currentTimeMillis(), so the max-read-time and drain-timeout tests
depended on wall-clock timing. On a slow or loaded runner the loop could
break before the first channel.read, leaving Mockito stubs unused and
failing with UnnecessaryStubbingException.

Inject a java.time.Clock into FileReader (public constructor defaults to
Clock.systemDefaultZone(), package-private constructor accepts a Clock),
matching the existing pattern in ServiceMapStatefulProcessor,
OTelApmServiceMapProcessor, and PipelineShutdown. Tests now drive time
with a mocked Clock: the read stub advances the fake clock past the
deadline after delivering data once, so exactly one read happens
deterministically. Removes all Thread.sleep and wall-clock assumptions
from these tests.

FileReaderPoolTest bookkeeping tests submitted readers to a real thread
pool where the reader completed on a background thread and mutated the
active/pending counts before the assertions ran. Add
createPoolWithNonRunningExecutor so queueing, promotion, and close logic
run synchronously. Tests that verify execution keep the real executor
and await via verify(..., timeout(...)).

Signed-off-by: Srikanth Padakanti <srikanth_padakanti@apple.com>

@lawofcycles lawofcycles left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks for taking this on. The injected Clock and non running executor make the tests deterministic. LGTM.

@dlvenable
dlvenable merged commit 817201e into opensearch-project:main Jul 21, 2026
72 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Flaky file-source unit tests fail the Gradle Build workflow

4 participants